Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(vep_parser): use nested schema for insilico predictors #789

Merged
merged 4 commits into from
Sep 26, 2024
Merged

fix(vep_parser): use nested schema for insilico predictors #789

merged 4 commits into from
Sep 26, 2024

Conversation

project-defiant
Copy link
Contributor

@project-defiant project-defiant commented Sep 25, 2024 •
edited
Loading

✨ Context

This PR closes #3546. More context in the issue.

🛠 What does this PR implement

  • Addition of helper function to extract nested StructType.
  • Pass correct schema for inSilicoPredictors field.
  • Addition of data case variants with multiple (2) elements in colocalised_variants field in following combination - [(rs123, None), (rs234, rs345)]

🙈 Missing

🚦 Before submitting

  • Do these changes cover one single feature (one change at a time)?
  • Did you read the contributor guideline?
  • Did you make sure to update the documentation with your changes?
  • Did you make sure there is no commented out code in this PR?
  • Did you follow conventional commits standards in PR title and commit messages?
  • Did you make sure the branch is up-to-date with the dev branch?
  • Did you write any new necessary tests?
  • Did you make sure the changes pass local tests (make test)?
  • Did you make sure the changes pass pre-commit rules (e.g poetry run pre-commit run --all-files)?

@github-actions github-actions bot added bug Something isn't working size-S Datasource labels Sep 25, 2024
@github-actions github-actions bot added size-M and removed size-S labels Sep 26, 2024
@project-defiant project-defiant marked this pull request as ready for review September 26, 2024 11:20
DSuveges
DSuveges approved these changes Sep 26, 2024
View reviewed changes
Copy link
Contributor

@DSuveges DSuveges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks OK, except this line: expected_schema: t.ArrayType | t.StructType | Column | str, See my comment, which is rather a question than a blocker.

@project-defiant project-defiant merged commit 51125c7 into dev Sep 26, 2024
5 checks passed
@DSuveges DSuveges deleted the variant-index-schema-mismatch branch September 26, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DSuveges DSuveges DSuveges approved these changes

Assignees
No one assigned
Labels
bug Something isn't working Datasource size-M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

VariantEffectPredictor outputs variant_index with incorrect schema
2 participants
@project-defiant @DSuveges